Fix what only worked on Linux, and a CRLF level file bug - #21
Conversation
The macOS branch of ResourceManager::setupDataPath() does not compile: it builds the bundle path with applePath + "/", where applePath is a char array and "/" is another array, so it is adding a pointer to a pointer. Whatever else is true of the macOS build, it has not been attempted for a long time. The editor's load and save dialogs start in $HOME, which on Windows is only set if somebody has set it, so they opened on nothing. They now start in the folder the player's own levels are saved to, which ResourceManager already resolves per platform, and which is where a level being loaded or saved from the editor belongs anyway. That was getEnv()'s only caller, and its comment already said it should be doing something else on Windows. Neither of these is verified on the platform it concerns. There is no macOS in the build system beyond a single if(WIN32 OR APPLE), and the Windows CI is an AppVeyor file pinned to a branch this fork does not have. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit db2e230)
A level file written on Windows does not load anywhere else. Its lines end with a carriage return before the line feed, and only Windows takes that back off when reading, so everywhere else the carriage return stays at the end of the line. There it makes the last value on the line, or a section marker standing alone, into something the parser does not recognise, and loading fails outright: verified here by converting a level to CRLF, which turns into "The level file can't be loaded". Helper::readFile now strips it, which covers every level and every config file since they all come through there. The same level loads cleanly afterwards, as do all twelve config files converted the same way. Then the Windows specific parts: The window icon is installed with SetClassLong and a handle cast to LONG. A handle is 64 bits wide in a 64 bit build and LONG stays 32, so that does not compile there. SetClassLongPtr is the same call in a 32 bit build. The Discord link ran xdg-open, which is the freedesktop way and exists on neither of the other two. It now uses ShellExecute on Windows and open on macOS. The editor's file lists asked whether a name starts with a dot to decide whether a file is hidden. On Windows that is an attribute of the file rather than anything in its name, and the code that would have read it was commented out, so the "show hidden files" checkbox did nothing. It now reads the attribute, which needs the whole path rather than the file name. setupOgreResources treated a path as absolute if it starts with '/', which no absolute Windows path does. And the two places that make sure a folder ends with a separator only accepted '/', so a path ending in a backslash got a slash added after it. Only the level file fix is verified: it is the only one of these that can be reached from Linux. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit f45b427)
|
MacOs , MacOs did anyone get the game running on that platform ? |
| #endif | ||
| // The result is worth looking at only to say so: there is nothing to fall back on, | ||
| // and the game is on its way out by the time this runs. | ||
| if(std::system(command.c_str()) != 0) |
There was a problem hiding this comment.
Maybe I am not too smart, but this branch will not compile on WINDOWS OS, because std::string command would end up being not declared anywhere .......
There was a problem hiding this comment.
Claude reported that compiling on Windows would require more extensive code changes. I told him to stop. Do you want to go on?
There was a problem hiding this comment.
Hmm I have my local code augmentation for my Windows installment in here, otherwise I wouldn't ship my binary version of ODP for Windows. Just for the sake of sanity , move the declaration of std::string command above the preprocessor '#if' ... I will cope with rest. BTW: are you aware of this little site : https://opendungeons.org/ ? ( with binary for W included ) .
There was a problem hiding this comment.
No, I wasn't aware of that site. I've asked Claude to fix building for Windows and test building for Mac. After he finishes, I will push the changes here.
There was a problem hiding this comment.
Claude has now verified this directly: it compile-checked every translation unit of the game for Windows with MinGW-w64 (GCC 16, both x86_64 and i686) against real Windows headers plus Ogre 13.6.5 / CEGUI / SFML 2.6 headers. AdvertMode.cpp compiles cleanly as it is: command is only declared and used inside the non-Windows #else branch — on Windows the call goes through ShellExecuteA instead, so nothing ends up undeclared. If you would still prefer std::string command hoisted above the #if for readability, happy to move it.
Two commits are now pushed to this branch:
- f876104 — the two remaining fixes needed to compile on macOS (verified: the game now compiles and links on a Mac, against Ogre 13.6.5 / CEGUI / SFML 2.6.2 / OIS 1.5.1 built from the versions pinned in snap/snapcraft.yaml);
- 9ccaa13 — one real Windows issue the sweep did find:
#define NOMINMAXcollides with MinGW's libstdc++, which predefines it, and that warning is fatal under the project's default-Werror. Both defines are now guarded with#ifndef.
Caveats: this was per-file compilation with MinGW, not a full Windows link, and the two StackTraceWin* files could not be checked here (they need bfd.h / MSVC).
There was a problem hiding this comment.
YEah I usually build my W binary bundle with MSVC ...
There was a problem hiding this comment.
I don't have Windows. Cannot test it.
Helper::toString calls with a size_t argument are ambiguous on macOS for the same reason they already were on OpenBSD: size_t is unsigned long, which is neither of the fixed-width 64 bit types there. Enable the same size_t overload on Apple. StackTraceUnix relies on struct sigcontext and the deprecated ucontext routines, neither of which macOS provides; give macOS the stub stack trace instead. Verified: the full game now compiles and links on macOS (Ogre 13.6.5, CEGUI, SFML 2.6.2, OIS 1.5.1 built from the versions pinned in snap/snapcraft.yaml). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
MinGW's libstdc++ already defines NOMINMAX, so defining it again warns, and the game compiles with warnings treated as errors by default. Only define NOMINMAX and WIN32_LEAN_AND_MEAN when nothing else has. With this, every game translation unit compiles for Windows: checked with MinGW-w64 GCC (x86_64 and i686) against real Windows headers, Ogre 13.6.5, CEGUI and SFML 2.6. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
On Apple the data path came only from the bundle layout, and nothing at all set the plugins.cfg path: outside a real .app bundle the game could not find its data and never loaded a render system. The current-folder overrides (data and plugins.cfg in "." win over the installed ones) and the plugins.cfg resolution were sitting in the Windows-and-Linux branch even though they are not platform specific; run them on every platform. Verified on macOS: the game now starts, loads all its data and runs a level in --server mode. (Graphics could not be exercised on the build machine, a VM with no GL driver; the rendering path is unchanged and assumes working drivers.) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Partially, yes — and it's on record in #31: I built the tree on macOS ( What nobody has done yet is run the full graphical game on a Mac — that additionally needs Ogre and CEGUI built for macOS, and I only had the machine long enough to validate the headless side. The fixes in this PR (the 🤖 Generated with Claude Code |
Fixes for everything that only ever worked on Linux, plus one level-file bug it hid:
Helper::readFilenow strips it, covering every level and config file.ResourceManager::setupDataPath()did not compile (char-array + char-array pointer arithmetic).$HOME, unset on Windows; they now start in the per-platform user levels folder.SetClassLongwith a handle cast toLONG, which does not compile in a 64-bit Windows build; nowSetClassLongPtr.xdg-openunconditionally; nowShellExecuteon Windows andopenon macOS.setupOgreResourcestreated any path not starting with/as relative, and folder-separator checks only accepted/.One hunk of the original second commit is not here: the separator fix inside
setupDefaultDataPath(), a function this branch does not have — it is carried by the user-data-folder PR instead.Note for merge order: trivial adjacent-lines conflicts with the user-data-folder PR (
ResourceManager.cpp) and the editor-improvements PR (EditorMode.cpp, the removedgetEnv()region).Split out of #16 so each topic can be reviewed on its own. Merging all of the split PRs reproduces the tree of #16 exactly.
🤖 Generated with Claude Code